CI Commands

The following CI workflows run automatically on every push and pull request:

The following commands can be used by maintainers to trigger additional tests that require access to secrets:

Note: Only repository maintainers (OWNER association) can trigger /metaci commands. After commenting the command, a maintainer must also navigate to the Actions tab and approve the deployment to the graph-api-access environment before the jobs will run. See the approval guidelines for what to approve or reject.

@claude

Claude finished @luccabb's task in 2m 23s —— View job

Code Review

• Gather context and read CLAUDE.md
• Read changed files (runtime.py, test_runtime.py)
• Review code and provide feedback

Overall this is a well-structured abstraction that genuinely reduces boilerplate. The dataclass + context manager pattern is clean, test coverage is solid, and the finish() method works correctly (calling sys.exit inside the with body means __exit__ is properly invoked). I found one correctness bug and a few smaller issues below.

Bug: sys.exit() inside __enter__ silently skips telemetry and output

runtime.py lines 97–103

if self.killswitch_getter():
    self.exit_code = ExitCode.OK
    self.logger.info(...)
    sys.exit(0)   # ← raises SystemExit from inside __enter__

When sys.exit(0) is called in __enter__, Python's with statement does not call __exit__ — __exit__ is only invoked when __enter__ returns successfully. This means self._stack.__exit__ is never called, so TelemetryContext.__exit__ (which writes telemetry records) and OutputContext.__exit__ (which prints the check result) are silently skipped.

This is a behavioral regression from the original code in check_sensors.py, where the killswitch check lives inside the with ExitStack() block — meaning SystemExit propagates through the with block body, the ExitStack is properly torn down, and both contexts get their __exit__ called.

The simplest fix is to drain the stack before exiting:

if self.killswitch_getter():
    self.exit_code = ExitCode.OK
    self.logger.info(
        "%s is disabled by killswitch", self.health_check_name.value
    )
    self._stack.__exit__(None, None, None)  # flush telemetry + output
    sys.exit(0)

Both TelemetryContext.__exit__ and OutputContext.__exit__ handle (None, None, None) correctly (they read from get_exit_code_msg() which captures self.exit_code, already set to ExitCode.OK above). Fix this →

Missing test coverage for the above bug

test_runtime.py lines 66–83 (test_killswitch_enabled_exits_ok)

The test verifies SystemExit(0) is raised but does not assert that TelemetryContext.__exit__ and OutputContext.__exit__ were called. Adding assertions for both mock_telem_instance.__exit__ and mock_output_instance.__exit__ (similar to test_telemetry_and_output_contexts_entered) would have caught the bug above. Fix this →

Minor: direct __enter__/__exit__ calls on _stack warrant a comment

runtime.py lines 72–73

self._stack = ExitStack()
self._stack.__enter__()

Calling dunder methods directly is unusual and can confuse readers. A brief comment like # Use ExitStack manually so it spans both __enter__ and __exit__ would make the intent clear.

Nit: missing docstrings on public class and finish()

HealthCheckRuntime and finish() have no docstrings. Given that this is intended as a shared utility that other check authors will use, a short class-level docstring and a finish() docstring (noting that it calls sys.exit and is therefore NoReturn) would help discoverability.

Nit: mixed logging style

runtime.py line 63 uses an f-string:

self.logger.warning(f"Could not get gpu_node_id, likely not a GPU host: {e}")

while the surrounding logger.info calls at lines 52–57 use %s formatting. The existing check_sensors.py uses the same f-string for this specific warning, so it's not wrong — just inconsistent within the file.

Summary